Fix missing last opened result due to race condition from plugins that call hide directly#4408
Fix missing last opened result due to race condition from plugins that call hide directly#4408
Conversation
|
🥷 Code experts: Jack251970 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA reordering of history-recording logic in MainViewModel.OpenResultAsync to execute immediately after result validation and before window hiding or result execution, rather than after execution. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Follow on from #4375 with issue #4374
Problem:
Some Sys plugin results (e.g 'Settings') do not show up in 'Empty Last Query' query style mode after they are selected, and would only show when re-queried or clearing of a new query.
Cause:
When plugin actions call Context.API.HideMainWindow() directly it does a re-query before the result is added to last history items and consequently the second Hide() call at the end of OpenResultAsync(string) would not trigger a re-query because QueryText is already "", so QueryText != queryText is false. Without a re-query the newly added last history item does not get added to result list display.
Fix:
Move _history.Add(result) to immediately after the result == null check, before ExecuteAsync(ActionContext) is called, guaranteeing that history is updated before any possible Hide() invocation — whether triggered from inside the plugin action or from the explicit Hide() call afterward.
Tested:
Summary by cubic
Fixes a race that caused the last selected system plugin result (e.g., “Settings”) to not appear in “Empty Last Query” mode. We now record the history entry before executing the action, so plugin-triggered re-queries don’t drop it.
OpenResultAsync, add the selected result to history beforeExecuteAsyncand anyHide()call; applies only to query results (not context menu/history).lastHistoryIndex = 1at the time of the early history write to ensure correct positioning in history._history.Addblock afterExecuteAsync, avoiding missed updates and duplicate logic.Written for commit a250e79. Summary will update on new commits.